Bitfinex: Fix WS trade processing #1754
Conversation
* Add handling for funding trades Fixes thrasher-corp#1746
gbjk
force-pushed
the
bugfix/bitfinex_ws_trade
branch
from
89cbb66 to
0dd865c
Compare
It's been a year, and I'm still getting caught out by govet demanding I don't shadow a var I was deliberately shadowing. Made worse by an increase in clashes with stylecheck when they both want opposite things on the same line.
gbjk
force-pushed
the
bugfix/bitfinex_ws_trade
branch
from
0dd865c to
cb416cc
Compare
exchanges/bitfinex/bitfinex_test.go
Outdated
| // TestWsCancelOrder dials websocket, sends modify request. | ||
| func TestWsUpdateOrder(t *testing.T) { | ||
| // TestWSCancelOrder dials websocket, sends modify request. | ||
| func TestWSUpdateOrder(t *testing.T) { |
There was a problem hiding this comment.
| func TestWSUpdateOrder(t *testing.T) { | |
| func TestWSModifyOrder(t *testing.T) { |
There was a problem hiding this comment.
DriveBy hit a few other naming fails whilst there.
Fixed gbjk@c7db5d8a2
| var ( | ||
| errParsingWSField = errors.New("error parsing WS field") | ||
| ) |
There was a problem hiding this comment.
| var ( | |
| errParsingWSField = errors.New("error parsing WS field") | |
| ) | |
| var errParsingWSField = errors.New("error parsing WS field") |
There was a problem hiding this comment.
Not so convinced, oddly, since it feels like this is a block of error vars, that happens to only contain one.
I want devs to add to it, essentially.
Did linter or editor highlight this?
| return errors.New("unable to type assert trade amount") | ||
| if t.Amount < 0 { | ||
| t.Side = order.Sell | ||
| t.Amount *= -1 |
There was a problem hiding this comment.
| t.Amount *= -1 | |
| t.Amount = math.Abs(t.Amount) |
| if !ok { | ||
| return errors.New("unable to type assert trade price") | ||
| t := &wsTrade{} | ||
| if err := json.Unmarshal(v, &[]any{&t.ID, &t.Timestamp, &t.Amount, &t.Price, &t.Period}); err != nil { |
There was a problem hiding this comment.
Suggestion: can define an UnmarshalJSON method with this and use the type wsTrade here and below in handleWSPublicTradeUpdate.
There was a problem hiding this comment.
Oh, yeah, that's really cute, thank you 🙏
I've put it in _types since it's a micro-method and there's nowhere moar organic to put it, other than maybe in _websocket next to the funcs that'll use it.
Fixed gbjk@4d3a7e0a4
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1754 +/- ##
==========================================
+ Coverage 37.13% 37.40% +0.26%
==========================================
Files 414 415 +1
Lines 180198 178586 -1612
==========================================
- Hits 66925 66807 -118
+ Misses 105413 103990 -1423
+ Partials 7860 7789 -71
|
exchange Bitfinex websocket error - unhandled WS trade update event:TestWsinstead ofTestWSFixes #1746
Progresses #1323 with a prototype
Type of change